-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
(maint) Log version and level at startup #599
Conversation
CLA signed by all contributors. |
Default output now is
|
d8ed9a2
to
3c75020
Compare
exe/main.cc
Outdated
LOG_DEBUG("pxp-agent logging has been initialized"); | ||
std::string version, loglevel; | ||
std::tie(version, loglevel) = Configuration::Instance().setupLogging(); | ||
LOG_INFO("pxp-agent {1} started at {2} level", version, loglevel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest returning only the log level from the Configuration::Instance().setupLogging()
method and using the PXP_AGENT_VERSION
here directly. The rationale being that the PXP_AGENT_VERSION
define is relevant to all of the pxp-agent code not just logging, so it seems inappropriate to return its value from the logging setup function. Moreover by simplifying the setupLogging()
's return value we'll be able to avoid having to pack/unpack the two values to/from the std::pair
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
lib/src/configuration.cc
Outdated
if (!log_on_stdout) { | ||
// Configure platform-specific things for file logging | ||
// NB: we do that after setting up lth_log in order to log in | ||
// case of failure | ||
configure_platform_file_logging(); | ||
} | ||
|
||
return std::make_pair(PXP_AGENT_VERSION, loglevel); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As suggested above: Return just the log level from the method.
Logs pxp-agent version and configured log level as soon as logging is configured. Clarifies when versions change in logging, which makes debugging issues easier.
3c75020
to
10fa306
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Logs pxp-agent version and configured log level as soon as logging is
configured. Clarifies when versions change in logging, which makes
debugging issues easier.